Indicate nodes not supported by current event + node folders#1156
Indicate nodes not supported by current event + node folders#1156
Conversation
Nickelony
left a comment
There was a problem hiding this comment.
Looks good! I'll poke Copilot as well to review just in case.
There was a problem hiding this comment.
Pull request overview
Adds metadata-driven support/unsupported event type tagging for visual scripting nodes, and surfaces a UI warning when a node is used under an incompatible event type.
Changes:
- Parse new
!Supported/!Unsupportedmetadata tags intoNodeFunctiondefinitions. - Add support checks (
IsUnsupported) toNodeFunctionand propagate current event type into the node editor UI. - Document the new metadata tags in the TEN node catalog Readme.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| TombLib/TombLib/Utils/ScriptingUtils.cs | Parses !Supported / !Unsupported tags into event type lists. |
| TombLib/TombLib/LevelData/VisualScripting/TriggerNodeEnumerations.cs | Stores per-node supported/unsupported event lists and exposes IsUnsupported. |
| TombLib/TombLib/Catalogs/TEN Node Catalogs/Readme.md | Documents the new metadata tag syntax and available event types. |
| TombLib/TombLib.Forms/Controls/VisualScripting/NodeEditor.cs | Displays a warning indicator/message when a node is incompatible with the current event type. |
| TombEditor/Forms/FormEventSetEditor.cs | Propagates event type changes into TriggerManager. |
| TombEditor/Controls/TriggerManager.cs | Exposes EventType and forwards it to the embedded node editor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- TombEditor/Forms/FormEventSetEditor.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -159,6 +154,8 @@ protected override void Dispose(bool disposing) | |||
|
|
|||
| if (DialogResult == DialogResult.Cancel) | |||
There was a problem hiding this comment.
Dispose currently restores state only when DialogResult == Cancel, but closing the window via the titlebar X typically leaves DialogResult as None, which will fall into the else branch and commit folder/order changes. If the intended behavior is to apply changes only on OK, change this to restore state for any non-OK close (DialogResult != DialogResult.OK) and only sync/apply on OK.
| if (DialogResult == DialogResult.Cancel) | |
| if (DialogResult != DialogResult.OK) |
| public bool GenericMode => GlobalMode || _instance == null; | ||
|
|
||
| private HashSet<string> _backupCollapsedFolders; | ||
| private HashSet<string> _сollapsedFolders => GlobalMode ? _editor.Level.Settings.CollapsedGlobalEventSetFolders : _editor.Level.Settings.CollapsedVolumeEventSetFolders; |
There was a problem hiding this comment.
The identifier _сollapsedFolders appears to use a non-ASCII Cyrillic 'с' character, which is very hard to spot and makes searching/refactoring error-prone. Rename it to an ASCII _collapsedFolders (and update references) to avoid future maintenance issues.
| private HashSet<string> _сollapsedFolders => GlobalMode ? _editor.Level.Settings.CollapsedGlobalEventSetFolders : _editor.Level.Settings.CollapsedVolumeEventSetFolders; | |
| private HashSet<string> _collapsedFolders => GlobalMode ? _editor.Level.Settings.CollapsedGlobalEventSetFolders : _editor.Level.Settings.CollapsedVolumeEventSetFolders; |
| var firstNonDrag = allVisible.FirstOrDefault(n => !_dragNodes.Contains(n)); | ||
| if (firstNonDrag != null) | ||
| { | ||
| var firstRect = GetNodeFullRowArea(firstNonDrag); | ||
| if (OffsetMousePosition.Y <= firstRect.Top) | ||
| { | ||
| // Mouse is above the first node — insert before it. | ||
| hitNode = firstNonDrag; | ||
| position = DropPosition.Before; | ||
| indicatorY = firstRect.Top; | ||
| } | ||
| else | ||
| { | ||
| // Mouse is below all nodes — append to root. | ||
| position = DropPosition.After; | ||
| indicatorY = ContentSize.Height; | ||
| } |
There was a problem hiding this comment.
HandleDrag() uses GetAllNodes() (which returns all nodes, including those under collapsed parents) and then the fallback firstNonDrag logic can pick a non-visible descendant. In that case GetNodeFullRowArea() returns an invalid rect (-1s), and the “mouse above first node” / “append” fallback can behave incorrectly. Fix by iterating only visible nodes for hit-testing/fallback (e.g., build a list by traversing only expanded branches, or filter nodes by checking all ancestors are expanded).
| public HashSet<string> CollapsedGlobalEventSetFolders { get; set; } = new HashSet<string>(); | ||
| public HashSet<string> CollapsedVolumeEventSetFolders { get; set; } = new HashSet<string>(); |
There was a problem hiding this comment.
LevelSettings.Clone() uses MemberwiseClone(), so these new HashSet<string> instances will be shared between the original settings and any clones (e.g., forms that work on a cloned LevelSettings). That can cause edits to collapsed folder state in a cloned settings instance to mutate the original. Fix by deep-copying both sets in Clone() (e.g., new HashSet<string>(CollapsedGlobalEventSetFolders) and same for volume).

Adds an indication that a node is not supported by the current event type:
To define whether node is supported or not supported by a particular event type, define new metadata entries:
-- !Unsupported "OnLoop" "OnLevelStart"-- !Supported "OnVolumeEnter"Additionally, adds support for folders in the node list.